Skip to content

Conversation

@CZEMacLeod
Copy link
Contributor

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Complete Cache/CacheDependency implementation to cover additional scenarios
System.Web.Caching.Cache - Support for dependencies (implemented via ChangeMonitors)
System.Web.Caching.Cache - Add missing Insert method
System.Web.Caching.CacheDependency - Add additional constructor overloads (and correct accessibility of 'empty' constructor)
System.Web.Caching.CacheDependency - Fix issue when start date is not supplied
Add tests for dependent files, and for dependent items
Add tests for cache insert
Fix issue when a CacheDependency has no changeMonitors

Addresses #347 and #350

…ath to ensure ChangeMonitors are always handled.

To support custom CacheDependency types that might not use change monitors under the surface, always call GetChangeMonitor and add it to the policy's change monitors.
Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I like it

@twsouthwick twsouthwick merged commit c9f45ce into dotnet:main Jun 5, 2023
@CZEMacLeod
Copy link
Contributor Author

If anyone needs AggregateCacheDependency in the future, I do have that code too.
https://github.com/CZEMacLeod/systemweb-adapters/tree/czemacleod/aggregatecache

@CZEMacLeod CZEMacLeod deleted the czemacleod/cache-1.2 branch June 6, 2023 00:04
@twsouthwick
Copy link
Member

If anyone needs AggregateCacheDependency in the future, I do have that code too. https://github.com/CZEMacLeod/systemweb-adapters/tree/czemacleod/aggregatecache

Feel free to open a PR for that - if you've needed it, it's likely others will as well. Thanks for your contributions here @CZEMacLeod !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants